Skip to content

Test improvement for OID cast index scan optimization#4873

Merged
tanscorpio7 merged 3 commits into
babelfish-for-postgresql:BABEL_6_X_DEVfrom
amazon-aurora:6x_cleanup_pr
Jun 22, 2026
Merged

Test improvement for OID cast index scan optimization#4873
tanscorpio7 merged 3 commits into
babelfish-for-postgresql:BABEL_6_X_DEVfrom
amazon-aurora:6x_cleanup_pr

Conversation

@RuchaSK1

@RuchaSK1 RuchaSK1 commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Cherry-picked from : #4872

This PR addresses review feedback from the OID cast index scan optimization PR that was merged earlier:

  1. Include ordering: Adjusted as per alphabetical position
  2. Removed InsertExecContext typedef and function declarations from pltsql.h that were included as part of the cherry-pick from 6x. - Not applicable here
  3. Volatile function test: Added a test case verifying that the contain_volatile_functions guard correctly prevents index optimization when the comparison value is volatile (e.g., random()). The test disables sequential scan to prove the hook definitively blocks the index path rather than the planner simply preferring a different plan on cost.

Regarding other AI review comments:

  • 500 schemas (Issue 3): The count was determined through testing to ensure consistent Index Scan plans across both local and CI environments. Reducing it causes plan instability. Additionally, it serves as a stress test that helps surface stale-stats issues in upgrade paths. If it causes timeout issues in future, the count can be reduced.
  • EXPLAIN fragility (Issue 4): Acknowledged. Plan shapes may change across PostgreSQL major versions and will be updated if needed.

Issues Resolved

Task: BABEL-6814
Authored-by: Rucha Kulkarni ruchask@amazon.com
Signed-off-by: Rucha Kulkarni ruchask@amazon.com

Test Scenarios Covered

  • Use case based - Yes

  • Boundary conditions -

  • Arbitrary inputs -

  • Negative test cases - Yes

  • Minor version upgrade tests -

  • Major version upgrade tests -

  • Performance tests -

  • Tooling impact -

  • Client tests -

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Rucha Kulkarni added 3 commits June 22, 2026 14:40
Test improvement for OID cast index scan optimization
6752ded 
Signed-off-by: Rucha Kulkarni <ruchask@amazon.com>
Correcting output as per PG18 change
c2fef49 
Signed-off-by: Rucha Kulkarni <ruchask@amazon.com>
Empty Commit
a4bfd53 
Signed-off-by: Rucha Kulkarni <ruchask@amazon.com>
@RuchaSK1

RuchaSK1 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/code-review

kuntalghosh
kuntalghosh approved these changes Jun 22, 2026
View reviewed changes

@kuntalghosh kuntalghosh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Code Review

Verdict: APPROVE

Based on the provided code changes, here are my feedback and suggestions:

This PR addresses review feedback from the OID cast index scan optimization (PR #4868) with two clean changes:

  1. Include reordering in hooks.c — Pure alphabetical sorting of #include directives. No headers were added or removed; the set of included headers is identical before and after. No executable code was changed.

  2. Volatile function test (Test 23) — Well-constructed regression test that disables sequential scan, queries with random()::int (a volatile expression), and verifies the planner produces a "Seq Scan (Disabled: true)" rather than an Index Scan. This proves the contain_volatile_functions guard in the hook correctly prevents the OID cast index optimization when the comparison value is volatile.

No blocking issues found.

Issue 1 (Advisory) - PR Description: Empty test scenario checklist:

The "Test Scenarios Covered" section in the PR template has all bullet items left blank. While the description body adequately explains the test being added (volatile function guard verification), filling in at least the "Use case based" bullet would improve clarity for future readers.

Notes

  • Security review completed on hooks.c: 0 findings. The change is a pure include reordering with no security impact.
  • The test correctly restores enable_seqscan to on after the test, preventing state leakage to subsequent tests.
  • Both standard and parallel_query expected output files are updated consistently.
  • Cherry-picked from PR #4872 targeting a different branch — base branch BABEL_6_X_DEV is appropriate.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.

@tanscorpio7 tanscorpio7 merged commit 9982e51 into babelfish-for-postgresql:BABEL_6_X_DEV Jun 22, 2026
49 checks passed
@tanscorpio7 tanscorpio7 deleted the 6x_cleanup_pr branch June 22, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kuntalghosh kuntalghosh kuntalghosh approved these changes
@tanscorpio7 tanscorpio7 tanscorpio7 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RuchaSK1 @kuntalghosh @tanscorpio7